Werner Koch [Mon, 18 Mar 2019 12:07:14 +0000 (13:07 +0100)]
gpg: Allow import of PGP desktop exported secret keys.
* g10/import.c (NODE_TRANSFER_SECKEY): New.
(import): Add attic kludge.
(transfer_secret_keys): Add arg only_marked.
(resync_sec_with_pub_keyblock): Return removed seckeys via new arg
r_removedsecs.
(import_secret_one): New arg r_secattic. Change to take ownership of
arg keyblock. Implement extra secret key import logic. Factor some
code out to ...
(do_transfer): New.
(import_matching_seckeys): New.
--
The PGP desktops exported secret keys are really stupid. And they
even a have kind of exception in rfc4880 which does not rule that
out (section 11.2):
[...] Implementations SHOULD include self-signatures on any user
IDs and subkeys, as this allows for a complete public key to be
automatically extracted from the transferable secret key.
Implementations MAY choose to omit the self-signatures, especially
if a transferable public key accompanies the transferable secret
key.
Now if they would only put the public key before the secret
key. Anyway we now have a workaround for that ugliness.
Werner Koch [Fri, 15 Mar 2019 18:50:37 +0000 (19:50 +0100)]
gpg: Avoid importing secret keys if the keyblock is not valid.
* g10/keydb.h (struct kbnode_struct): Replace unused field RECNO by
new field TAG.
* g10/kbnode.c (alloc_node): Change accordingly.
* g10/import.c (import_one): Add arg r_valid.
(sec_to_pub_keyblock): Set tags.
(resync_sec_with_pub_keyblock): New.
(import_secret_one): Change return code to gpg_error_t. Return an
error code if sec_to_pub_keyblock failed. Resync secret keyblock.
--
When importing an invalid secret key ring for example without key
binding signatures or no UIDs, gpg used to let gpg-agent store the
secret keys anyway. This is clearly a bug because the diagnostics
before claimed that for example the subkeys have been skipped.
Importing the secret key parameters then anyway is surprising in
particular because a gpg -k does not show the key. After importing
the public key the secret keys suddenly showed up.
This changes the behaviour of
GnuPG-bug-id: 4392
to me more consistent but is not a solution to the actual bug.
Caution: The ecc.scm test now fails because two of the sample keys
don't have binding signatures.
Werner Koch [Fri, 15 Mar 2019 18:11:32 +0000 (19:11 +0100)]
gpg: During secret key import print "sec" instead of "pub".
* g10/keyedit.c (show_basic_key_info): New arg 'print_sec'. Remove
useless code for "sub" and "ssb".
* g10/import.c (import_one): Pass FROM_SK to show_basic_key_info. Do
not print the first keyinfo in FROM_SK mode.
printing.
--
Werner Koch [Tue, 5 Mar 2019 11:39:11 +0000 (12:39 +0100)]
gpg: Make invalid primary key algos obvious in key listings.
* g10/keylist.c (print_key_line): Print a warning for invalid algos.
--
Non-OpenPGP compliant keys now show a warning flag on the sec or pub
line like in:
gpg: can't encode a 256 bit MD into a 88 bits frame, algo=8
sec cv25519 2019-01-30 [INVALID_ALGO] 4239F3D606A19258E7A88C3F9A3F4F909C5034C5
uid [ultimate] ffffff
Instead of showing the usage flags "[CE]". Without this patch only
the error message is printed and the reason for it was not immediately
obvious (cv25519 is encryption only but we always consider the primary
key as having the "C" flag).
Before this change, running "make check" accesses USB device by
scdaemon on host computer. If there is any smartcard/token available,
it may affect test results. Because default key choice depends on
smartcard/token availability now and existing tests have nothing about
testing smartcard/token, disabling scdaemon is good.
* doc/gpgv.texi: Improve documentation for keyring choices
--
From the existing documentation, it's not clear whether the default
keyring will always be mixed into the set of keyrings, or whether it
will be skipped if a --keyring is present. The updated text here
attempts to describe the keyring selection logic more completely.
Werner Koch [Thu, 28 Feb 2019 13:43:42 +0000 (14:43 +0100)]
sm: Don't mark a cert as de-vs compliant if it leads to SHA-1 sigs.
* sm/keylist.c (print_compliance_flags): Also check the digest_algo.
Add new arg 'cert'.
--
A certificate with algorithm sha1WithRSAEncryption can be de-vs
compliant (e.g. if the next in the chain used sha256WithRSAEncryption
to sign it and RSA is long enough) but flagging it as such is useless
because that certificate can't be used because it will create
signatures using the non-compliant SHA-1 algorithm.
Well, it could be used for encryption. But also evaluating the
key-usage flags here would make it harder for the user to understand
why certain certificates are listed as de-vs compliant and others are
not.
Werner Koch [Fri, 22 Feb 2019 13:09:02 +0000 (14:09 +0100)]
agent: Fix for suggested Libgcrypt use.
* agent/divert-scd.c (divert_pkdecrypt): Skip a flags parameter.
--
The libgcrypt docs say that a "flags" parameter should always be used
in the input of pkdecrypt. Thus we should allow that parameter also
when parsing an s-expression to figure out the algorithm for use with
scdaemon.
scd: Distinguish cancel by user and protocol error.
* scd/apdu.h (SW_HOST_CANCELLED): New.
* scd/apdu.c (host_sw_string): Support SW_HOST_CANCELLED.
(pcsc_error_to_sw): Return SW_HOST_CANCELLED for PCSC_E_CANCELLED.
* scd/iso7816.c (map_sw): Return GPG_ERR_INV_RESPONSE for
SW_HOST_ABORTED and GPG_ERR_CANCELED for SW_HOST_CANCELLED.
NIIBE Yutaka [Mon, 28 Jan 2019 03:58:13 +0000 (12:58 +0900)]
agent: Clear bogus pinentry cache, when it causes an error.
* agent/agent.h (PINENTRY_STATUS_*): Expose to public.
(struct pin_entry_info_s): Add status.
* agent/call-pinentry.c (agent_askpin): Clearing the ->status
before the loop, let the assuan_transact set ->status. When
failure with PINENTRY_STATUS_PASSWORD_FROM_CACHE, it returns
soon.
* agent/findkey.c (unprotect): Clear the pinentry cache,
when it causes an error.
Werner Koch [Tue, 22 Jan 2019 09:06:15 +0000 (10:06 +0100)]
gpg: Stop early when trying to create a primary Elgamal key.
* g10/misc.c (openpgp_pk_test_algo2): Add extra check.
--
The problem is that --key-gen --batch with a parameter file didn't
detect that Elgamal is not capable of signing and so an error was only
triggered at the time the self-signature was created. See the code
comment for details.
If for example a server was built without the development packages of
the compression libraries installed, the server will not be able to
decrypt a request. In theory this can't happen due to the preference
system but it is just to easy to create the server's key using a
different version of gpg and then use gpg-wks-server built
differently.
For the short messages we exchange compression is not really required
and thus we better do without to make the system more robust.
Apparently there is no clean way to configure this from ./configure,
and upstream is deprecating gpg-zip anyway. So just force-set tar to
be manually "tar" (meaning, that we should look in the $PATH at
runtime).
See also https://dev.gnupg.org/T4251 and https://bugs.debian.org/913582
gpg: Prefer SHA-512 and SHA-384 in personal-digest-preferences.
* g10/keygen.c (keygen_set_std_prefs): prefer SHA-512
and SHA-384 by default.
--
In 8ede3ae29a39641a2f98ad9a4cf61ea99085a892, upstream changed the
defaults for --default-preference-list to advertise a preference for
SHA-512, without touching --personal-digest-preferences. This makes
the same change for --personal-digest-preferences, since every modern
OpenPGP library supports them all.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic update-defaults
Gbp-Pq: Name gpg-Prefer-SHA-512-and-SHA-384-in-personal-digest.patch
gpg: Default to SHA-512 for all signature types on RSA keys.
* g10/main.h (DEFAULT_DIGEST_ALGO): Use SHA512 instead of SHA256 in
--gnupg mode (leave strict RFC and PGP modes alone).
* configure.ac: Do not allow disabling sha512.
* g10/misc.c (map_md_openpgp_to_gcry): Always support SHA512.
--
SHA512 is more performant on most 64-bit platforms than SHA256, and
offers a better security margin. It is also widely implemented.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic update-defaults
Gbp-Pq: Name gpg-Default-to-SHA-512-for-all-signature-types-on-RS.patch
Since libassuan 2.1.0, cancellation command "CAN" is handled within
the library, by assuan_transact. So, cancel_inquire just caused
spurious "CAN" command to scdaemon which resulted an error.
* g10/main.h (DEFAULT_CIPHER_ALGO): Prefer AES256 by default.
--
It's 2017, and pretty much everyone has AES-256 available. Symmetric
crypto is also rarely the bottleneck (asymmetric crypto is much more
expensive). AES-256 provides some level of protection against
large-scale decryption efforts, and longer key lengths provide a hedge
against unforseen cryptanalysis.
* agent/command.c (hlp_genkey): update help text to suggest the use of
3072 bits.
* doc/wks.texi: Make example match default generation.
* g10/keygen.c (DEFAULT_STD_KEY_PARAM): update to
rsa3072/cert,sign+rsa3072/encr, and fix neighboring comment,
(gen_rsa, get_keysize_range): update default from 2048 to 3072).
* g10/keyid.c (pubkey_string): update comment so that first example
is the default 3072-bit RSA.
--
3072-bit RSA is widely considered to be 128-bit-equivalent security.
This is a sensible default in 2017.
agent: Avoid scheduled checks on socket when inotify is working.
* agent/gpg-agent.c (handle_connections): When inotify is working, we
do not need to schedule a timer to evaluate whether we control our own
socket or not.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic gpg-agent-idling
Gbp-Pq: Name agent-Avoid-scheduled-checks-on-socket-when-inotify-.patch
* agent/gpg-agent.c (need_tick): Evaluate whether the short-phase
handle_tick() is needed.
(handle_connections): On each cycle of the select loop, adjust whether
we should call handle_tick() or not.
(start_connection_thread_ssh, do_start_connection_thread): Signal the
main loop when the child terminates.
* agent/call-scd.c (start_scd): Call interrupt_main_thread_loop() once
the scdaemon thread context has started up.
--
With this change, an idle gpg-agent that has no scdaemon running only
wakes up once a minute (to check_own_socket).
Thanks to Ian Jackson and NIIBE Yutaka who helped me improve some of
the blocking and corner cases.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic gpg-agent-idling
Gbp-Pq: Name agent-Avoid-tight-timer-tick-when-possible.patch
agent: Allow threads to interrupt main select loop with SIGCONT.
* agent/gpg-agent.c (interrupt_main_thread_loop): New function on
non-windows platforms, allows other threads to interrupt the main loop
if there's something that the main loop might be interested in.
--
For example, the main loop might be interested in changes in program
state that affect the timers it expects to see.
I don't know how to do this on Windows platforms, but i welcome any
proposed improvements.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic gpg-agent-idling
Gbp-Pq: Name agent-Allow-threads-to-interrupt-main-select-loop-wi.patch
agent/gpg-agent.c (handle_tick): Remove intermittent call to
check_own_socket.
(tv_is_set): Add inline helper function for readability.
(handle_connections) Create general table of pending scheduled
timeouts.
--
handle_tick() does fine-grained, rapid activity. check_own_socket()
is supposed to happen at a different interval.
Mixing the two of them makes it a requirement that one interval be a
multiple of the other, which isn't ideal if there are different delay
strategies that we might want in the future.
Creating an extensible regular timer framework in handle_connections
should make it possible to have any number of cadenced timers fire
regularly, without requiring that they happen in cadences related to
each other.
It should also make it possible to dynamically change the cadence of
any regularly-scheduled timeout.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic gpg-agent-idling
Gbp-Pq: Name agent-Create-framework-of-scheduled-timers.patch
* dirmngr/dirmngr.c (housekeeping_thread): Avoid automatically
checking upstream's software database. In Debian, software updates
should be handled by the distro mechanism, and additional upstream
checks only confuse the user.
* doc/dirmngr.texi: document that --allow-version-check does nothing.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic dirmngr-idling
Gbp-Pq: Name dirmngr-Avoid-automatically-checking-upstream-swdb.patch
* dirmngr/ks-engine-hkp.c (host_is_alive): New function. Test whether
host is alive and resurrects it if it has been dead long enough.
(select_random_host, map_host, ks_hkp_mark_host): Use host_is_alive
instead of testing hostinfo_t->dead directly.
(ks_hkp_housekeeping): Remove function, no longer needed.
* dirmngr/dirmngr.c (housekeeping_thread): Remove call to
ks_hkp_housekeeping.
--
Rather than resurrecting hosts upon scheduled resurrection times, test
whether hosts should be resurrected as they're inspected for being
dead. This removes the need for explicit housekeeping, and makes host
resurrections happen "just in time", rather than being clustered on
HOUSEKEEPING_INTERVAL seconds.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic dirmngr-idling
Gbp-Pq: Name dirmngr-Avoid-need-for-hkp-housekeeping.patch
dirmngr: hkp: Avoid potential race condition when some hosts die.
* dirmngr/ks-engine-hkp.c (select_random_host): Use atomic pass
through the host table instead of risking out-of-bounds write.
--
Multiple threads may write to hosttable[x]->dead while
select_random_host() is running. For example, a housekeeping thread
might clear the ->dead bit on some entries, or another connection to
dirmngr might manually mark a host as alive.
If one or more hosts are resurrected between the two loops over a
given table in select_random_host(), then the allocation of tbl might
not be large enough, resulting in a write past the end of tbl on the
second loop.
This change collapses the two loops into a single loop to avoid this
discrepancy: each host's "dead" bit is now only checked once.
As Werner points out, this isn't currently strictly necessary, since
npth will not switch threads unless a blocking system call is made,
and no blocking system call is made in these two loops.
However, in a subsequent change in this series, we will call a
function in this loop, and that function may sometimes write(2), or
call other functions, which may themselves block. Keeping this as a
single-pass loop avoids the need to keep track of what might block and
what might not.
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Gbp-Pq: Topic dirmngr-idling
Gbp-Pq: Name dirmngr-hkp-Avoid-potential-race-condition-when-some.patch
This avoids needing to setgid gpg-agent. It probably doesn't defend
against all possible attacks, but it defends against one specific (and
easy) one. If there are other protections we should do them too.
This will make it slightly harder to debug the agent because the
normal user won't be able to attach gdb to it directly while it runs.
The remaining options for debugging are:
* launch the agent from gdb directly
* connect gdb to a running agent as the superuser
Upstream bug: https://dev.gnupg.org/T1211
Gbp-Pq: Topic block-ptrace-on-secret-daemons
Gbp-Pq: Name Avoid-simple-memory-dumps-via-ptrace.patch
upstream ships doc/defsincdate in its tarballs. but doc/Makefile.am
tries to rewrite doc/defsincdate if it notices that any of the files
have been modified more recently, and it does so assuming that we're
running from a git repo.
However, we'd rather ship the documents cleanly without regenerating
defsincdate -- we don't have a git repo available (debian builds from
upstream tarballs) and any changes to the texinfo files (e.g. from
debian/patches/) might result in different dates on the files than we
expect after they're applied by dpkg or quilt or whatever, which makes
the datestamp unreproducible.
Gbp-Pq: Topic debian-packaging
Gbp-Pq: Name avoid-regenerating-defsincdate-use-shipped-file.patch
Using autoreconf against the source as distributed in tarball form
invariably results in a package that thinks it's a "beta" package,
which produces the "THIS IS A DEVELOPMENT VERSION" warning string.
since we use dh_autoreconf, i need this patch to avoid producing
builds that announce themselves as DEVELOPMENT VERSIONs.
* drop unneeded patch for printing revocation certificates
* backport bugfix and stability patches from upstream 2.2.13
* backport bugfix and stability patches from upstream 2.2.14
* backport documentation, stability, ssh, and WKD patches from upstream 2.2.15
* backport documentation and bugfix patches from upstream 2.2.16
* import bugfixes and cleanup around secret key handling from 2.2.14
* backport bugfixes, documentation, WKD, and keyserver fixes from 2.2.17
* import efficiency and security fixes from upstream STABLE-BRANCH-2-2
* avoid using SKS pool CA unless the keyserver is hkps.pool.sks-keyservers.net
* drop import-clean from default keyserver options, to avoid data loss
* use keys.openpgp.org as the default keyserver
* enable merging certificate updates even if update has no user ID
* update Vcs-Git: to point to debian/buster branch
* Adopt migrate-pubring-from-classic-gpg robustness fixes (Closes: #931385)
* add new CI test: debian/tests/simple-tests
* debian/tests/gpgv-win32: make arch-specific (Closes: #905563)